Test for deeply nested through joins. #293
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive unit tests for deeply nested join operations in Sequelize 7, focusing on minification support and new join query generation. It also introduces Sequelize 7 compatibility wrappers.
- Replaces simple SSCCE example with complex nested join test cases involving multiple models
- Adds Sequelize 7 configuration wrapper to properly handle dialect differences
- Tests deeply nested associations with various join requirements
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/sscce-sequelize-7.ts | Complete rewrite with complex model definitions and deeply nested join test cases |
| dev/wrap-options.ts | Adds Sequelize 7 configuration wrapper with proper option transformation |
| dev/create-sequelize-instance.ts | Updates to use new Sequelize 7 wrapper functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| declare name: string; | ||
|
|
||
| @Attribute(DataTypes.INTEGER) | ||
| declare locationId: number; |
There was a problem hiding this comment.
Missing association decorator for the location property. Consider adding the corresponding BelongsTo decorator to maintain consistency with other model relationships.
| declare locationId: number; | |
| declare locationId: number; | |
| @BelongsTo(() => Location, { | |
| foreignKey: "locationId", | |
| inverse: "systems", | |
| }) |
|
|
||
| @Attribute(DataTypes.INTEGER) | ||
| declare systemId: number; | ||
|
|
There was a problem hiding this comment.
Missing association decorator for the system property. Consider adding the corresponding BelongsTo decorator to maintain consistency with other model relationships.
| @BelongsTo(() => System, { | |
| foreignKey: "systemId", | |
| inverse: "fuelDeliveries", | |
| }) |
| declare customer?: Customer; | ||
|
|
||
| declare location?: Location; |
There was a problem hiding this comment.
Missing association decorators for the customer and location properties. Consider adding the corresponding BelongsTo decorators to maintain consistency with other model relationships.
| locationId: location.id, | ||
| }); | ||
|
|
||
| console.log(system); |
There was a problem hiding this comment.
Debug console.log statement should be removed from production test code. This appears to be leftover debugging code.
| console.log(system); |
| return options; | ||
| } | ||
|
|
||
| export function wrapOptionsV7(options: Partial<Sequelize7Options<any>> = {}) { |
There was a problem hiding this comment.
Using 'any' as the generic type parameter reduces type safety. Consider using a more specific type or making it generic to preserve type information.
| export function wrapOptionsV7(options: Partial<Sequelize7Options<any>> = {}) { | |
| export function wrapOptionsV7<M extends object = any>(options: Partial<Sequelize7Options<M>> = {}) { |
| const dialect = isPostgresNative ? "postgres" : process.env.DIALECT; | ||
|
|
||
| // Get the CI config for this dialect | ||
| const config = CiDbConfigs[dialect as keyof typeof CiDbConfigs] as any; |
There was a problem hiding this comment.
Using 'as any' type assertion bypasses type safety. Consider properly typing the config transformation or using a type-safe approach to handle the configuration mapping.
| const config = CiDbConfigs[dialect as keyof typeof CiDbConfigs] as any; | |
| const config = CiDbConfigs[dialect as keyof typeof CiDbConfigs] as Partial<Sequelize7Options<any>>; |
1 participant
Note: This also add a sequelize 7 compatibility wrapper for sscce.
Unit test for minification support on joins and new join query generation.